feat: add Castle.customize/1 as the release integration API - #27
Merged
Conversation
A project makes a Mix release Castle-capable by piping its release options through `Castle.customize/1` and names nothing else. The splice of Forecastle's pre- and post-assembly steps around `:assemble` is `Forecastle.steps/1`, reused rather than reimplemented, so what the pair does at assembly time can change without every consumer's `mix.exs` changing with it. Steps a project has of its own keep their order, and the documented form is the lazy `fn -> ... end`, because `mix.exs` is evaluated on loads that have yet to build `castle` itself. castle#12. `:steps` defaults to `[:assemble, :tar]` where Mix's own default is `[:assemble]`. `Castle.unpack/1` reaches `unpack_release/1`, which reads `releases/<name>-<vsn>.tar.gz`, so a version assembled without `:tar` can never be handed to a running deployment - which is the whole of what Castle is for. A `:steps` list that *is* given without `:tar` is built as it was written, with a warning. Refusing it would refuse a working configuration: the deployment an upgrade is installed onto needs no tarball of its own, so a base deployment shipped as a directory is legitimate, and a function step in the list may be packing one anyway. Honouring it silently leaves the cost to be met as a `bin/castle unpack` that cannot find a file, which names a missing tarball rather than the option that did not ask for one. So the build says what is missing, and says only that. A list with no `:assemble`, or a `:steps` that is not a list, comes back untouched for `mix release` to refuse itself - it validates the option and names it, and a second implementation of that rule could only drift from it. `customize/1` carries the `@doc` and `@spec` the rest of the API still lacks (castle#11), including the four things it deliberately does not set: the `:appup` project key and compiler, a relup from `mix forecastle.relup`, `include_executables_for: [:unix]`, and the optional `rel/env.sh.eex`. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C4oaMvbR1cbxrZBj8qwkqN
It said the release "is never packed" and that "nothing can install this
version", then acknowledged the counterexample in a trailing sentence without
retracting either claim. All the check can see is one atom missing from the
list as given; whether an archive appears is not visible to it, because a
function step later in the list can pack one itself or add :tar to the steps
still to run - %Mix.Release{} carries those remaining steps precisely so a
step can.
That combination is the worst available: a definite diagnosis on the error
channel, sending an operator to investigate a packaging failure that may not
exist, with the retraction buried after it. The warning now states the
condition it observed and what follows unless something else packs the
archive, and asserts nothing else.
The test for it asserts the absence of both retracted claims as well as the
presence of the conditional, so a message that dropped the consequence
altogether fails too.
Co-Authored-By: Claude <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #12. Targets
release/1.0.0.One public call makes a Mix release Castle-capable, so a consumer's
mix.exsnever names Forecastle:Deliberately thin
Forecastle.steps/1already does the whole splice — finds:assemble, puts the pre-assembly step before it and the post-assembly step after, leaves everything around them where it was — and has shipped since 0.1.3. So this isKeyword.update(opts, :steps, Forecastle.steps(@default_steps), &spliced/1)and there is no second implementation of the splice. That the function is Forecastle's is the one thingcustomize/1exists to keep out of a consumer's project.The issue's "preserve existing custom steps and their ordering" therefore comes for free rather than being new code.
The
:tardecision, and a claim it correctedAbsent
:stepsdefaults to[:assemble, :tar], not Mix's[:assemble].:stepssupplied without:taris honoured, with a build-time warning — not refused. The reasoning matters because the obvious argument for refusing is wrong::release_handler'sdo_unpack_release/4readsfilename:join(RelDir, ReleaseName ++ ".tar.gz"), whereReleaseNameis the version being installed. A running release needs no tarball of its own, so a base deployment shipped as a directory — a container image — built withsteps: [:assemble]is a perfectly good Castle deployment. Refusing it would refuse a working configuration. The accurate claim is narrower: such a release can never be the target of an upgrade.And the warning asserts no verdict. It can see one atom missing from the list as given; it cannot see whether an archive appears anyway, because a function step later in the list can pack one itself or add
:tarto the steps still to run —%Mix.Release{}carries the remaining steps precisely so a step can. An earlier revision said the release "is never packed" and "nothing can install this version", then acknowledged the counterexample in a trailing sentence without retracting either: a definite diagnosis on the error channel, sending an operator after a packaging failure that may not exist. It now states the condition observed and what follows unless something else packs it.Two cases the issue did not name
:assemble— passed through silently.Mix.Release.validate_steps!/1requires exactly one, refuses the release and names the option, and Mix validates after the lazy release function runs, so its refusal is always downstream. A second implementation of Mix's rule could only drift from it.:stepsvalue that is not a list — handed back, becauseForecastle.steps/1guards onis_list/1and aFunctionClauseErrorout of it would surface a module the project never mentioned, which is exactly whatcustomize/1exists to prevent. Pinned by a test so it is not a dead branch.What it does not cover
Documented in the
@docrather than left to be discovered: the:appupproject key withcompilers: Mix.compilers() ++ [:appup], runningmix forecastle.relupas a build step,include_executables_for: [:unix](Windows is unsupported), and the optionalrel/env.sh.eex. The lazyfn -> … endform is documented too, with the reason — it is what guarantees the dependency is compiled before the customisation runs.Scope
@docand@specforcustomize/1only. The module's@moduledocis still themix newplaceholder and the rest of the API is undocumented: that is #11, and doing it here would mean editing #11's own limitation entry. Forecastle is untouched — in particular its fixture stays on explicitpre_assemble/post_assemblesteps, because Forecastle has to remain testable without Castle's API.Tests
142 → 154, twelve new cases. Every assertion is on the whole
:stepslist in order, with surrounding custom steps bound as values so identity comparison pins them — a case that only checked:stepswas present would pass against a broken splice. The missing-:tardecision is pinned in both halves: the list is built as written and the warning fires, so acustomize/1that quietly appended:tarfails.async: false, because the warning is observed throughMix.Shell.Processand Mix's shell is one setting for the node.Verification
mix precommitgreen — 154 tests, credo--strictclean — andcompile --warnings-as-errorsclean under Elixir 1.20.3/OTP 28, where the type inference is what caught a provably-dead branch earlier in this series.mix docsalso caught a factual error while this was being written: the@docclaimed:tarisMix.Release.make_tar/1. It is not —make_tar/1isdefpinMix.Tasks.Releaseand not callable. Corrected in the doc, the code comment andAGENTS.md; the argument survives in the weaker, true form, that a function step can pack a tarball itself.